Skip to content

gh-135846: Add zstd dependency to Android build script #136253

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 3 commits into from
Jul 9, 2025
Merged

gh-135846: Add zstd dependency to Android build script #136253

merged 3 commits into from
Jul 9, 2025
+1 −1

Conversation

emmatyping
Copy link
Member

@emmatyping emmatyping commented Jul 3, 2025 •
edited by bedevere-app bot
Loading

This PR adds libzstd to the Android platform build script. The source deps build script was added in beeware/cpython-android-source-deps#5.

The host configure found the libraries to enable _zstd:

checking for libzstd >= 1.4.5... yes
...
checking for stdlib extension module _zstd... yes

@bedevere-app bedevere-app bot mentioned this pull request Jul 3, 2025
Open
@emmatyping
Copy link
Member Author

!buildbot Android

@bedevere-bot
Copy link

🤖 New build scheduled with the buildbot fleet by @emmatyping for commit 8fe0fd9 🤖

Results will be shown at:

https://buildbot.python.org/all/#/grid?branch=refs%2Fpull%2F136253%2Fmerge

The command will test the builders whose names match following regular expression: Android

The builders matched are:

  • AMD64 Android PR
  • aarch64 Android PR

@emmatyping
Copy link
Member Author

Hmm, looks like libzstd.so.1 wasn't copied into the build package..

0:14:31 [491/491] test_zstd
test_zstd skipped -- dlopen failed: library "libzstd.so.1" not found: needed by /data/data/org.python.testbed/files/python/lib/python3.15/lib-dynload/_zstd.cpython-315-aarch64-linux-android.so in namespace clns-6
0:14:31 [491/491] test_zstd skipped

@emmatyping
Copy link
Member Author

Ah, I probably need to add libzstd here:

cpython/Android/android.py

Line 651 in da79ac9

("lib", ["engines-3", "libcrypto*.so", "libpython*", "libsqlite*",

@emmatyping
Copy link
Member Author

Hm, that isn't needed for bz2/lzma...

@mhsmith mhsmith added the needs backport to 3.14 bugs and security fixes label Jul 3, 2025
@mhsmith
Copy link
Member

mhsmith commented Jul 3, 2025 •
edited
Loading

Hm, that isn't needed for bz2/lzma...

That's because their cpython-android-source-deps releases only include static libraries. We should probably update the zstd build script to do the same. In that case, nothing would need to change in this PR except the zstd build number.

But FYI, if we did add any dynamic libraries, then we'd need to update not only the packaging script you linked above, but also the testbed here:

cpython/Android/testbed/app/build.gradle.kts

Lines 234 to 239 in b499105

generateTask(variant, variant.sources.jniLibs!!) {
for ((prefix, abi) in abis.entries) {
into(abi) {
from("$prefix/lib")
include("libpython*.*.so")
include("lib*_python.so")

@emmatyping
Copy link
Member Author

That's because their cpython-android-source-deps releases only include static libraries. We should probably update the zstd build script to do the same.

Oh! I didn't realize that, interesting. I'll make a PR now with that change.

@emmatyping
Copy link
Member Author

beeware/cpython-android-source-deps#6

mhsmith
mhsmith reviewed Jul 9, 2025
View reviewed changes
@mhsmith
Copy link
Member

mhsmith commented Jul 9, 2025

!buildbot android

@bedevere-bot
Copy link

🤖 New build scheduled with the buildbot fleet by @mhsmith for commit 26ddec4 🤖

Results will be shown at:

https://buildbot.python.org/all/#/grid?branch=refs%2Fpull%2F136253%2Fmerge

The command will test the builders whose names match following regular expression: android

The builders matched are:

  • aarch64 Android PR
  • AMD64 Android PR

@emmatyping
Copy link
Member Author

Hm, the buildbot failure is in test_tmpfile, which doesn't interact with zstd. Seems like an unrelated failure.

mhsmith
mhsmith approved these changes Jul 9, 2025
View reviewed changes
Copy link
Member

@mhsmith mhsmith left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Android will be broken on the main branch until #136430 is merged, but the zstd and zipfile tests are all passing.

@freakboy3742: please merge.

@freakboy3742
Copy link
Contributor

!buildbot android

@bedevere-bot
Copy link

🤖 New build scheduled with the buildbot fleet by @freakboy3742 for commit 869feb1 🤖

Results will be shown at:

https://buildbot.python.org/all/#/grid?branch=refs%2Fpull%2F136253%2Fmerge

The command will test the builders whose names match following regular expression: android

The builders matched are:

  • aarch64 Android PR
  • AMD64 Android PR

@freakboy3742 freakboy3742 merged commit 61dd9fd into python:main Jul 9, 2025
41 checks passed
@miss-islington-app
Copy link

Thanks @emmatyping for the PR, and @freakboy3742 for merging it 🌮🎉.. I'm working now to backport this PR to: 3.14.
🐍🍒⛏🤖

miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Jul 9, 2025
@emmatyping @mhsmith @miss-islington
pythongh-135846: Add zstd dependency to Android build script (pythonG…
3458965 
…H-136253)

Adds zstd to the Android build process.

---------
(cherry picked from commit 61dd9fdad729fe02d91c03804659f7d0c5a89276)

Co-authored-by: Emma Smith <[email protected]>
Co-authored-by: Malcolm Smith <[email protected]>
@bedevere-app
Copy link

bedevere-app bot commented Jul 9, 2025

GH-136491 is a backport of this pull request to the 3.14 branch.

@bedevere-app bedevere-app bot removed the needs backport to 3.14 bugs and security fixes label Jul 9, 2025
@freakboy3742
Copy link
Contributor

Thanks @emmatyping and @mhsmith for your work on this (both here, and in the dependency repos)

freakboy3742 pushed a commit that referenced this pull request Jul 10, 2025
@miss-islington @emmatyping @mhsmith
[3.14] gh-135846: Add zstd dependency to Android build script (GH-136253
7937f16 
) (#136491)

Adds zstd to the Android build process.
(cherry picked from commit 61dd9fd)

Co-authored-by: Emma Smith <[email protected]>
Co-authored-by: Malcolm Smith <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mhsmith mhsmith mhsmith approved these changes

@freakboy3742 freakboy3742 Awaiting requested review from freakboy3742 freakboy3742 is a code owner

Assignees
No one assigned
Labels
skip news
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@emmatyping @bedevere-bot @mhsmith @freakboy3742